Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expose parsed date parts. Invalid date if 'a' token parsed but no date parts parsed. #3045

Closed
wants to merge 1 commit into from

Conversation

maggiepint
Copy link
Member

PR resolves #3037 and #2986

The date parts parsed are exposed in parsingFlags().parsedDateParts array. Array only has date parts at indexes parsed.

Because actual date parts parsed is being tracked now, it is possible to modify isValid to evaluate as false if no date parts are being parsed, but meridiem indicator was parsed.
This resolves a current behavior where junk data can parse valid if it has the letters 'a' or 'p' in it (or whatever indicates meridiem in the specific locale).
Current behavior:

moment('asdfgh', 'hh:mm a').isValid()
true

@mattjohnsonpint
Copy link
Contributor

Thanks Maggie. I think this will help a lot!

@@ -5,14 +5,18 @@ import getParsingFlags from '../create/parsing-flags';
export function isValid(m) {
if (m._isValid == null) {
var flags = getParsingFlags(m);
var parsedParts = flags.parsedDateParts.some(function (i) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don't use some. Its not supported on IE8 :) You can put it in utils, and use the Array.prototype if exists (but don't modify Array)

@ichernev
Copy link
Contributor

@maggiepint wow, that is a neat fix! We've had this for forever. Just get rid of Array.prototype.some and its square.

@maggiepint
Copy link
Member Author

Oh IE8. You are 👎

… flags. Change isValid to cause date to be invalid if only token parsed is meridiem.
@maggiepint
Copy link
Member Author

Now works with technology ancient history.

@maggiepint
Copy link
Member Author

@ichernev I did the pseudo-polyfill the same way you do them since I figured that's what you would want. I'm curious though, what is your reasoning for using that particular pattern, as opposed to a regular polyfill, or even just creating a more plain wrapper function that takes the array as a parameter?

To be clear, I know reasons in general why people are opposed to polyfills. I just want to know your reasons.

@ichernev
Copy link
Contributor

Merged in e7bb14a

@ichernev ichernev closed this Apr 16, 2016
ichernev added a commit that referenced this pull request Apr 16, 2016
Expose parsed date parts. Invalid date if 'a' token parsed but no date parts parsed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Modify parse flags to include parsed date parts
3 participants